Skip to content

Fix/nominal attr mapping - #40

Merged
diejdablju merged 5 commits into
mainfrom
fix/nominal-attr-mapping
Oct 7, 2026
Merged

diejdablju merged 5 commits into
mainfrom
fix/nominal-attr-mapping

Conversation

@BartPigula

Copy link
Copy Markdown
Collaborator

This PR incorporates fixes made in Java Rulekit in order to repair issues with nominal attributes mapping. Currently the mapping is updated for a given test set so that it agrees with mapping created during training.

Comment thread rulekit/_helpers.py Outdated
Object array safe to pass through JPype into DataTable.
"""
result = np.empty(values.shape, dtype=object)
for index, value in np.ndenumerate(values):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Martwi mnie wydajność tego, bo lecimy tutaj w pętli po każdej wartości ze zbioru danych. W sumie zbiory danych podawane na RuleKit nie są duże, wiec pewnie nie będzie to nawet zauważalne, ale zastanawia mnie czemu by nie zrobić np. tak:

def _to_java_object_array(values: np.ndarray) -> np.ndarray:
result = values.astype(object)
return np.where(pd.isna(result), None, result)

albo:

def _to_java_object_array(values: np.ndarray) -> np.ndarray:
result = values.astype(object)
result[pd.isna(result)] = None
return result

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

słuszna uwaga, poniosło mnie z tym fixem po najmniejszej linii :) wrzuciłem update

Comment thread rulekit/_operator.py
header = self.model._java_object.getTrainingHeader()
if header is not None:
try:
example_set = example_set.updateMapping(header)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co robi ta funkcja? Rozumiem, że to jest po to, że gdy w zbiorze treningowym mam np. atrybut rozmiar: big. small, medium to pod spodem jest to mapowane na: big: 0, small: 1, medium: 2 i jak podam inny zbiór z wartościami small, big, medium to dostaną one pod spodem inny mapping i przez to zostaną zwrócone złe wyniki i ten updateMapping to dopasowuje do zbioru treningowego?

Pytanie czy w takim razie w predict też nie musimy tego dodać jeżeli damy inny zbiór do predykcji? Tam też tworzymy nowy zbiór danych jak tutaj:
example_set = ExampleSetFactory(self._get_problem_type()).make(values)
return self.model._java_object.apply( # pylint: disable=protected-access
example_set
)

chyba, że w javie w apply to już jest dopasowywane?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tak, dokładnie - updateMapping po zmianach Adama to taki odpowiednik transform z sklearn. W predict nie trzeba wywoływać wprost, bo apply w klasie PredictionModel pod spodem wywołuje updateMapping(getTrainingHeader())

@diejdablju
diejdablju merged commit 66700be into main Oct 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants